[ENH] Move PET DICOM correspondences into term definitions #2298
Conversation
Add DICOM correspondence to term definition while we're here
effigies
added
PET
schema
effigies
requested review from
mnoergaard and
yarikoptic
and removed request for
erdalkaraca
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2298 +/- ##
=======================================
Coverage 82.81% 82.81%
=======================================
Files 22 22
Lines 1693 1693
=======================================
Hits 1402 1402
Misses 291 291 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
effigies
left a comment
effigies
left a comment
There was a problem hiding this comment.
Tracer DICOM tags were added by @bendhouseart in #1021, and signed off by @mnoergaard, @sappelhoff and @tsalo. The question of whether to put them in terms or in the table as addenda was addressed:
I'm not confident enough to say that these tags should belong in the
metadata.ymlas many of these terms are often more explicitly defined (or known....) by the experiment designer/performer or simply not present in the dicom headers. Adding them to the tables provides a bread crumb for a user to extract this info if they're not omnipotent concerning the experiment or the metadata.
Originally posted by @bendhouseart in #1021 (comment)
I'm okay removing them altogether, if that's the updated consensus, or putting them back (with comments) in metadata.yaml. From the perspective of someone reading the PET page, there is no difference; the difference is in the glossary or looking the term up in the schema directly.
Any updates to the text for clarity would be appreciated.
|
Can't say that I'm not pleased to see src/schema/rules/sidecars/pet.yaml slimmed down nearly 100 lines. Cheers 👍 |
4 participants
Follow-up to #2294.
This PR moves the
Corresponds to [DICOM Tag ...section intoobjects.metadataterms for PET. This excludes terms likeInstitutionName, which are recommended for many data types that are not converted from DICOM.Minor cleanups here:
BodyPartentry. This was probably just an error in schematization.InjectionStartandInjectionStop, change "with respect to"TimeZero"in the default unit seconds." to "with respect to"TimeZero", in seconds." BIDS does not support alternative units, so we can use simple language here.PharmaceuticalName. While worth pointing out, it doesn't need to be pointed out multiple times, and it seems reasonable to include this in the term definition itself.rules.sidecars.petwithout adding toobjects.metadatabecause they were already present in theobjects.metadataterm.